add a standalone dispose(), and make it work on every platform - #156
Open
hpoul wants to merge 2 commits into
Open
add a standalone dispose(), and make it work on every platform#156hpoul wants to merge 2 commits into
hpoul wants to merge 2 commits into
Conversation
getStorage resolves a name's StorageFileInitOptions once and then ignores them, so a store could not be reconfigured while the process lived — changing an authentication validity duration meant restarting the app. @luckyrat identified this in #138, following #75 and #137, and proposed deleteAndDispose. This implements the standalone dispose() instead: the use case is reconfiguring, and coupling it to delete made you destroy the stored secret to change a duration. delete() first if you want both. The method was already reachable natively and did not work. Linux had no handler, so `dispose` fell through to fl_method_not_implemented_response_new() and reached Dart as a MissingPluginException. darwin replied `true` from a no-op that never cleared `stores`, so the next init took its short circuit, discarded the freshly passed options and replied `false` — success at every step, with the file still on its original configuration. That silent one is why the load-bearing test asserts through forceInit rather than trusting a bool. Android reported the same operation by throwing NoSuchStorage on a repeat, the only backend where disposing twice was an error. It now returns whether there was anything to forget, which is the contract every platform now shares. win32 and web needed a fix that predates this API: `_initialized` was keyed by the bare name while BiometricStorageFile carries the prefixed one, so removing by the name dispose is handed would have matched nothing and quietly returned false forever. Both now key on the prefixed form. The stored content is deliberately untouched everywhere; this reconfigures rather than erases, and a test pins that. Verified: nine integration tests green on the iOS simulator, which compiles the shared darwin sources and uses a sandboxed keychain rather than the login one. Reintroducing the darwin no-op turned two of the three new tests red — the reinit one and the idempotence one, while the content-survives one correctly stayed green — and reverting restored them. flutter analyze --fatal-infos and flutter test clean in both packages. Linux and Windows are covered by CI. Breaking for platform implementations only: `dispose` is a new member on the BiometricStorage platform interface. No change for callers. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
hpoul
force-pushed
the
standalone-dispose
branch
from
August 25, 2026 19:19
8bc1457 to
870ed3e
Compare
There was a problem hiding this comment.
Pull request overview
Adds a new public API, BiometricStorageFile.dispose(), to forget a store’s runtime initialization (so the next getStorage can apply new StorageFileInitOptions) while deliberately leaving the stored secret intact. This fits the plugin’s cross-platform “single Dart surface, multiple backends” design by wiring the same dispose semantics through Dart, MethodChannel platforms, and the pure-Dart web/win32 implementations.
Changes:
- Add
BiometricStorageFile.dispose()plus a Dart-side “spent handle” guard so disposed handles consistently rejectread/write/deleteacross all platforms. - Implement/repair native + platform-layer dispose handling (Linux handler added; Android + darwin fixed to actually forget initialization; win32/web fix name-keying so dispose can find entries).
- Add tests (integration + unit) pinning: re-init after dispose, idempotent dispose, content preserved, and disposed-handle behavior.
Reviewed changes
Copilot reviewed 12 out of 13 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
lib/src/biometric_storage.dart |
Adds the platform-interface dispose method, MethodChannel implementation, and the BiometricStorageFile.dispose() API + disposed-handle guard. |
linux/biometric_storage_plugin.cc |
Implements a missing Linux dispose handler by removing the name from the initialization registry without deleting the secret. |
android/src/main/kotlin/design/codeux/biometric_storage/BiometricStoragePlugin.kt |
Makes Android dispose idempotent and returns a boolean indicating whether an initialization was forgotten. |
darwin/biometric_storage/Sources/biometric_storage/BiometricStorageImpl.swift |
Fixes darwin dispose from a no-op to actually removing the store entry, returning whether anything was removed. |
lib/src/biometric_storage_win32.dart |
Keys initialization tracking by prefixed name and implements dispose by forgetting that entry. |
lib/src/biometric_storage_web.dart |
Keys initialization tracking by prefixed name and implements dispose by forgetting that entry (leaving localStorage intact). |
example/integration_test/storage_test.dart |
Adds integration tests that prove dispose works (re-init, idempotency, content preserved, spent handle semantics). |
test/biometric_storage_test.dart |
Adds a MethodChannel-level unit test asserting the dispose call reaches the platform with the expected arguments (skipped on Windows). |
test/biometric_storage_win32_test.dart |
Adds win32-specific unit tests covering name-forgetting, content preservation, and spent-handle enforcement. |
example/lib/main.dart |
Adds a UI action to call dispose() and logs the result + expected post-dispose behavior. |
CHANGELOG.md |
Documents the new API, behavioral contracts, and platform fixes. |
pubspec.yaml |
Bumps version to 6.0.0-dev.5. |
example/pubspec.lock |
Updates the example’s locked version to 6.0.0-dev.5. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
hpoul
force-pushed
the
standalone-dispose
branch
from
August 25, 2026 19:35
870ed3e to
0db555b
Compare
Two findings from review, a failure CI caught that the review did not, and a footgun found while checking the fix. A disposed BiometricStorageFile documented that it must not be used again, and nothing enforced it — so the consequence of ignoring the doc differed by platform. Android and darwin resolve read, write and delete against their registry and fail once the entry is gone, while Linux, Windows and web answer from the name alone and carry on working. That is a mistake you make on one platform and discover on another. The guard now sits in Dart, so every backend answers alike: read, write and delete throw a StateError, and dispose deliberately does not, because disposing twice has to stay legal. The flag is set before the call is awaited, not after. That brick a handle whose native dispose then fails, which is worth it: Dart always sends `name`, so the argument-error paths are dead, while the race it closes is reachable by any caller who does not await dispose() and would otherwise slip a read into the window — succeeding on three platforms and failing on two, which is the divergence this guard exists to remove. Recovering costs one getStorage. dispose is scoped to the name, not to the handle: every handle is a view onto one name-keyed initialization, so a spent handle still forgets an initialization made after it. Documented and tested, because the tempting optimization — short-circuiting a repeat dispose on the local flag rather than asking the platform — would quietly make it handle-scoped and return a wrong answer. The `null reply is reported as nothing-to-forget` test was vacuous and is gone. No deployable combination replies null — the handlers that predate this either answer true, or throw, or are missing entirely, which surfaces as MissingPluginException — and the `?? false` it claimed to guard is enforced by the compiler anyway, since invokeMethod<bool> is typed Future<bool?>. The coercion stays, with a comment saying what it is actually for. The remaining dispose unit tests are skipped on a Windows host. They drive MethodChannelBiometricStorage, whose dispose asks _promptInfoForCurrentPlatform for a prompt it has no windows case for, so they failed there with "Unsupported Platform windows". Not reachable by users — the registrant installs Win32BiometricStoragePlugin — and biometric_storage_win32_test.dart covers dispose on that host. Adding the guard invalidated two things written alongside it: a win32 teardown that deleted through the handle it had just disposed, and the example's claim that read and write keep working afterwards. Both fixed. Verified by execution on all five backends. Eleven integration tests green on the Android emulator, the iOS simulator and macOS; Linux runs the same suite against real libsecret in CI, and Windows runs the win32 unit suite, which reaches the same dispose paths by a different route. Reintroducing the Android NoSuchStorage throw turned two red — the idempotence test and the guard test's closing assertion — and reverting restored them; the darwin no-op did the same earlier. Both mutation runs predate the last two tests, so they show those fixes are caught, not that every test in the suite is. macOS had never run this suite before, having been signing-blocked; a provisioning profile now exists. It writes to the real login keychain, so the run was bracketed by a keychain baseline — zero integration_ entries before and after, which is what says teardown cleans up rather than that the tests passed. No prompt appeared, which is what authenticationRequired false is for. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
hpoul
force-pushed
the
standalone-dispose
branch
from
August 25, 2026 19:40
0db555b to
1cdd305
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds
BiometricStorageFile.dispose(), which forgets a store's initialization without touching its content, so the nextgetStorageapplies a freshStorageFileInitOptions.Today the options for a name are resolved once and then ignored for the lifetime of the process, so a store cannot be reconfigured at runtime — changing an authentication validity duration means restarting the app.
Credit
@luckyrat identified this gap and proposed an API for it in #138, following #75 and the fork consolidation in #137. This implements the standalone
dispose()rather than that PR'sdeleteAndDispose: the use case is reconfiguring, and coupling it to deletion made you destroy the stored secret in order to change a duration.delete()thendispose()composes if you want both.Implemented fresh on current
mainrather than rebasing #138, which branched fromab59b2b(5.1.1-dev.2);lib/src/biometric_storage.darthas moved +329/-185 andexample/lib/main.dart+134 since. #138 also changed no native code, and the method did not actually work on three of the five backends.What was broken
disposewas already reachable natively. It did not work.fl_method_not_implemented_response_new()and reached Dart as aMissingPluginException.truefrom a no-op that never clearedstores, so the nextinittook its short circuit, discarded the freshly passed options and repliedfalse— reporting success at every step while leaving the file on its original configuration. This silent one is why the load-bearing test asserts throughforceInitrather than trusting a returned bool.NoSuchStorageon a repeat, the only backend where disposing twice was an error. It now reports whether there was anything to forget._initializedwas keyed by the bare name whileBiometricStorageFilecarries the prefixed one, so removing by the namedisposeis handed would have matched nothing and quietly returnedfalseforever. Both now key on the prefixed form.The stored content is deliberately untouched on every platform — this reconfigures rather than erases, and a test pins that.
Two contract decisions worth knowing
A disposed handle is spent.
read,writeanddeletethrow aStateError;disposeitself does not, so disposing twice stays legal. Without this the platforms disagreed — Android and darwin fail against their registry, Linux/Windows/web carry on from the name alone — which is a mistake you make on one platform and find on another. The flag is set before the call is awaited, so a failed dispose still spends the handle; the race that closes is reachable, the failure it risks is not.disposeis scoped to the name, not the handle. Every handle is a view onto one name-keyed initialization, so a spent handle still forgets an initialization created after it. Documented and tested, because short-circuiting a repeat dispose on the local flag would quietly make it handle-scoped and answer wrongly.Verification
Executed on all five backends — the eleven integration tests are the same suite everywhere, so the counts below are all of them:
NoSuchStoragethrow turned 2 redBiometricStorageImpl.swiftwith iOS)macOS had never run this suite before — it was signing-blocked, and a provisioning profile now exists. It writes to the real login keychain, so the run was checked against a before/after baseline: zero
integration_*entries either side, confirming teardown cleans up. No keychain prompt appeared, which is the point ofauthenticationRequired: false. What it adds beyond iOS is narrow but real: the#if os(macOS)messenger branch inBiometricStoragePlugin.swift, and a real keychain rather than a sandboxed simulator one.Two honest qualifications. The mutation runs predate the last two tests: the darwin and Android mutations were performed when the suite had 9 and 10 tests respectively, so they prove those fixes are caught, not that every current test is. And Windows never runs the integration suite — it runs
biometric_storage_win32_test.dart, which reaches the same dispose paths through the plugin directly rather than through the method channel.The dispose unit tests are skipped on a Windows host — they drive
MethodChannelBiometricStorage, which is unreachable there because the registrant installsWin32BiometricStoragePlugin;biometric_storage_win32_test.dartcovers dispose on that host.flutter analyze --fatal-infosandflutter testclean in both packages.Breaking
For platform implementations only:
disposeis a new member on theBiometricStorageplatform interface, so a third-party implementation extending it must add one. No change for callers of the package. Taken now deliberately, while 6.0.0 is still pre-release.Closes #138.